[feat](olap) Support lazy reading mode for pruned complex columns#59263
Open
mrhhsg wants to merge 8 commits into
Open
[feat](olap) Support lazy reading mode for pruned complex columns#59263mrhhsg wants to merge 8 commits into
mrhhsg wants to merge 8 commits into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
mrhhsg
force-pushed
the
opt_lazy_read
branch
from
5ff30aa to
b39fda3
Compare
Member
Author
|
run buildall |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
mrhhsg
force-pushed
the
opt_lazy_read
branch
from
b39fda3 to
c734a83
Compare
Member
Author
|
run buildall |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
mrhhsg
force-pushed
the
opt_lazy_read
branch
from
c734a83 to
ca5a57c
Compare
Member
Author
|
run buildall |
mrhhsg
force-pushed
the
opt_lazy_read
branch
from
ca5a57c to
1734d3f
Compare
Member
Author
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 34393 ms |
TPC-DS: Total hot run time: 174201 ms |
ClickBench: Total hot run time: 28 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
mrhhsg
force-pushed
the
opt_lazy_read
branch
from
1734d3f to
91706c6
Compare
Member
Author
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 35535 ms |
TPC-DS: Total hot run time: 175439 ms |
ClickBench: Total hot run time: 27.99 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Issue Number: None Related PR: apache#59263 Problem Summary: Pruned complex columns can separate predicate subcolumns from non-predicate subcolumns. Predicate branches must be read before filter evaluation, while non-predicate branches can be deferred until after filtering to avoid unnecessary IO and materialization. This change adds the scan and iterator state needed to keep predicate, lazy, meta-only, and skipped branches distinct for struct/map/array columns, recovers placeholder columns after lazy materialization, and keeps nested iterator reading flags explicit. It also avoids generating TopN runtime filters for unsupported complex order key types so BE does not receive invalid runtime predicate keys. Support lazy reading of non-predicate fields for pruned complex columns, and avoid unsupported complex TopN runtime filter keys. - Test: Unit Test / Build / Format - `./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.TopNRuntimeFilterTest#testNotUseTopNRfForUnsupportedComplexOrderKey` - `ninja -C be/ut_build_ASAN src/exec/CMakeFiles/Exec.dir/operator/olap_scan_operator.cpp.o src/exec/CMakeFiles/Exec.dir/scan/olap_scanner.cpp.o src/storage/CMakeFiles/Storage.dir/segment/column_reader.cpp.o src/storage/CMakeFiles/Storage.dir/segment/segment_iterator.cpp.o test/CMakeFiles/doris_be_test.dir/storage/segment/column_reader_test.cpp.o` - `build-support/clang-format.sh be/src/exec/operator/olap_scan_operator.cpp be/src/exec/operator/olap_scan_operator.h be/src/exec/scan/olap_scanner.cpp be/src/runtime/runtime_state.h be/src/storage/olap_common.h be/src/storage/segment/column_reader.cpp be/src/storage/segment/column_reader.h be/src/storage/segment/segment_iterator.cpp be/src/storage/segment/segment_iterator.h be/test/storage/segment/column_reader_test.cpp` - `git diff --check && git diff --cached --check` - Behavior changed: Yes. Non-predicate subcolumns of pruned complex columns can be read lazily, and unsupported complex TopN order keys no longer create runtime filters. - Does this need documentation: No
Issue Number: None Related PR: apache#59263 Problem Summary: Nested column pruning stripped predicate metadata paths when a non-predicate data path covered the same nested container for lazy materialization. A query that projected a nested map-array-struct field but filtered with cardinality(map['key']) could evaluate the predicate without value-array offsets and filter out matching rows. Keep predicate metadata paths covered by predicate-phase paths, preserve final predicate metadata paths even when lazy materialization covers the corresponding data path, and avoid forwarding current-level array metadata predicate paths to item iterators. Fix nested complex column pruning for predicates that need array/map metadata. - Test: - Build: `~/.codex/skills/doris-local-regression/scripts/doris-local-regression.sh --network 10.26.20.3/24 all -d nereids_rules_p0/column_pruning -s string_length_column_pruning` built FE/BE successfully. - Regression test: `~/.codex/skills/doris-local-regression/scripts/doris-local-regression.sh --network 10.26.20.3/24 start && ~/.codex/skills/doris-local-regression/scripts/doris-local-regression.sh --network 10.26.20.3/24 run -d nereids_rules_p0/column_pruning -s string_length_column_pruning`. - Code style: `build-support/clang-format.sh be/src/storage/segment/column_reader.cpp`, `build-support/check-format.sh be/src/storage/segment/column_reader.cpp`, `git diff --check`, and `git diff --cached --check`. - Static analysis: attempted `build-support/run-clang-tidy.sh --build-dir be/build_Release`, but it failed on existing analysis diagnostics outside the changed lines and a missing system `stddef.h` resource include. - Behavior changed: Yes. Nested predicate metadata paths are preserved so filtered lazy reads evaluate predicates with the required array/map metadata. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Clarify the NestedColumnPruning phase 1.5 comment so it matches the current predicate metadata path stripping logic. Predicate access paths are stripped only by predicate-phase paths, while all access paths are stripped by self-covering paths. ### Release note None ### Check List (For Author) - Test: No need to test (comment-only change); ran git diff --check and git diff --cached --check - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#59263 Problem Summary: Nested column pruning could treat metadata paths used by predicate evaluation as redundant when a covering data path also existed for final lazy materialization. This is unsafe because predicate evaluation still needs current-level metadata, such as OFFSET for cardinality()/length() and NULL for IS NULL, before row filtering. Removing these paths could make BE enter metadata-only modes incorrectly or forward current-level metadata paths to child iterators. This change keeps predicate metadata paths in the FE plan, removes the obsolete MetaPathStriper logic, and updates BE nested column iterators to consume current-level metadata paths at the correct iterator level without letting redundant metadata paths switch mixed data reads into meta-only mode. Regression expectations are updated to assert the retained metadata-path contract. ### Release note None ### Check List (For Author) - Test: - Regression test: `doris-local-regression --network 10.26.20.3/24 run -d nereids_rules_p0/column_pruning` - Manual test: `build-support/clang-format.sh` - Manual test: `git diff --check` - Behavior changed: No - Does this need documentation: No
Issue Number: None
Related PR: None
Problem Summary: Predicate evaluation can require nested access paths even when final materialization does not need any data access path at the current complex iterator level. The complex column iterators previously returned early when all access paths were empty, which ignored predicate-only metadata or child access paths. This could leave predicate-only nested columns unread in lazy read mode and produce incorrect predicate results for cases such as array element IS NULL predicates. This change only skips access-path setup when both final access paths and predicate access paths are empty.
None
- Test: Unit Test and Regression test
- Unit Test: ./run-be-ut.sh --run --filter=ColumnReaderTest.*
- Regression test: doris-local-regression --network 10.26.20.3/24 run -d nereids_rules_p0/column_pruning -s string_length_column_pruning
- Regression test: doris-local-regression --network 10.26.20.3/24 run -d datatype_p0/complex_types -s test_pruned_columns
- Regression test: doris-local-regression --network 10.26.20.3/24 run -d inverted_index_p0/array_contains -s test_index_compaction_null_arr
- Manual test: build-support/clang-format.sh
- Manual test: git diff --check
- Behavior changed: No
- Does this need documentation: No
Issue Number: None Related PR: apache#59263 Problem Summary: Struct lazy-read access-path routing could skip a child that only appeared in predicate access paths. For example, with projection path `s.a` and predicate path `s.b`, `StructFileColumnIterator::set_access_paths()` decided whether to route a child only from ordinary projection access paths, so child `b` was marked as skipped before its predicate path was forwarded. This change collects both projection and predicate subpaths for each child first, and routes the child when either side requires it. None - Test: Unit Test - `./run-be-ut.sh --run --filter=ColumnReaderTest.StructPredicateOnlyChildPathStillRoutesToChild:ColumnReaderTest.MapFullProjectionStillRoutesPredicateSubPaths:ColumnReaderTest.AccessPathsPropagatePredicateToChildren` - `./run-be-ut.sh --run --filter=ColumnReaderTest.*` - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Add focused coverage for ColumnReader nested iterator routing, including access-path modes, prefetcher collection, next_batch behavior, and read_by_rowids behavior for STRUCT/ARRAY/MAP nullable and meta-only cases. Extend the nested container offset pruning regression case to cover mixed empty/non-empty containers and predicate/output access-path combinations.
### Release note
None
### Check List (For Author)
- Test: Regression test / Unit Test
- DORIS_HOME=/mnt/disk7/hushenggang/doris-fix-spill ninja -C be/ut_build_ASAN test/CMakeFiles/doris_be_test.dir/storage/segment/column_reader_test.cpp.o
- DORIS_HOME=/mnt/disk7/hushenggang/doris-fix-spill ./run-be-ut.sh --run --filter=ColumnReaderTest.*
- doris-local-regression.sh --network 10.26.20.3/24 run -d nereids_rules_p0/column_pruning -s nested_container_offset_pruning -forceGenOut
- doris-local-regression.sh --network 10.26.20.3/24 run -d nereids_rules_p0/column_pruning -s nested_container_offset_pruning
- build-support/clang-format.sh be/test/storage/segment/column_reader_test.cpp
- build-support/check-format.sh
- git diff --check
- Behavior changed: No
- Does this need documentation: No
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
Member
Author
|
/review |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| order by 1, 2 limit 0, 20; | ||
| """ | ||
|
|
||
| sql "clean all profile" |
Contributor
There was a problem hiding this comment.
This global profile cleanup can make the new assertion interfere with other profile-based regression suites. clean all profile routes to ProfileManager.cleanProfile(), which clears the shared FE queryIdToProfileMap and queryIdToExecutionProfiles; this suite is declared only as suite("test_pruned_columns"), so it is not isolated from concurrent suites that may be polling /rest/v1/query_profile for their own UUID-tagged query. Since the code below already searches by lazyPrunedToken, please drop the global cleanup (or mark the suite nonConcurrent if clearing all profiles is required) so this new test does not cause unrelated profile assertions to flake.
|
|
||
| _recovery_from_place_holder_column(dst); | ||
|
|
||
| if (read_null_map_only()) { |
Contributor
There was a problem hiding this comment.
This NULL_MAP_ONLY rowid path can return the wrong null map for sparse rowid batches. In NULL_MAP_ONLY mode, seek_to_ordinal(rowids[total_read_count]) positions on the first requested row's page, but when that page has no null bitmap the branch below fills min(remaining, _page.remaining()) zeros without checking whether the next requested rowids are still covered by the current page. SegmentIterator::_read_columns_by_index() calls read_by_rowids() for non-contiguous batches, so a batch like [row in a no-null page, row in a later page that does contain a null] is consumed entirely there and the later null row is reported as non-null. Please mirror the _page.has_null / normal data path behavior by only advancing over requested rowids that belong to the current page before filling zeros.
| AccessPathInfo accessPathInfo = result.get(slot.getExprId().asInt()); | ||
| if (accessPathInfo != null) { | ||
| retainPredicatePathsInFinalAllAccessPaths( | ||
| addPredicatePathsToFinalAllAccessPaths( |
Contributor
There was a problem hiding this comment.
This changes the FE/BE access-path contract without a mixed-version guard. The new BE reader code knows how to handle allAccessPaths that contain both data paths and predicate metadata paths, but an old BE does not: its _check_and_set_meta_read_mode() switches to OFFSET_ONLY/NULL_MAP_ONLY as soon as it sees any current-level OFFSET or NULL subpath, and then skips the child iterators. During a rolling upgrade, a new FE can therefore send paths such as [s.NULL, s.city] (or map/array data paths plus parent metadata) to an old BE, which will default-fill or skip the projected child data. Please either keep the old stripped path shape until all selected BEs advertise support for mixed metadata/data access paths, or gate this new superset form on a BE capability/version.
Contributor
TPC-H: Total hot run time: 29681 ms |
Contributor
TPC-DS: Total hot run time: 172006 ms |
Contributor
ClickBench: Total hot run time: 25.2 s |
Member
Author
|
/review |
| sql "set enable_common_expr_pushdown = false" | ||
| sql "set enable_prune_nested_column = false" | ||
|
|
||
| qt_sql6_1 """ |
Contributor
There was a problem hiding this comment.
These two new result checks run after the suite disables both gates that are required to enter the lazy-pruned recovery path. Lines 270-271 set enable_common_expr_pushdown=false and enable_prune_nested_column=false, but the BE only adds a column to _support_lazy_read_pruned_columns when it is a common-expression column and nested pruning is enabled. That means qt_sql6_1 and qt_sql6_2 validate the normal reader, not the new lazy recovery for offset and reverse-order selections. Please keep these flags enabled through these two queries, or put the required SET_VAR settings on the queries and restore the session afterward.
| order by 1 limit 0, 20; | ||
| """ | ||
|
|
||
| Thread.sleep(1000) |
Contributor
There was a problem hiding this comment.
A fixed one-second sleep is not enough to make this profile assertion deterministic. The profile list exposes Profile Completion State, and FE reports RUNNING/COLLECTING until the BE fragment profiles have been merged; if this fetches the profile during that window, LazyReadPrunedTime can still be missing or zero even though the query eventually completes normally. Please poll the UUID-tagged profile until Profile Completion State == COMPLETE (and ideally until LazyReadPrunedTime is present) before asserting the timer value.
Member
Author
|
/review |
| void SetUp() override {} | ||
| void TearDown() override {} | ||
| void SetUp() override { | ||
| config::disable_storage_page_cache = true; |
Contributor
There was a problem hiding this comment.
This fixture now forces the process-wide config::disable_storage_page_cache flag to true, but TearDown() only removes the test directory and never restores the old value. doris_be_test runs storage tests in one process, and this flag defaults to false and controls both ColumnReader page-cache usage and segment ColumnIteratorOptions::use_page_cache, so any test that runs after this fixture can silently inherit page cache disabled and become order-dependent. Please save the previous value in the fixture and restore it in TearDown() or scope the override to the single file-backed test that needs it.
### What problem does this PR solve? Issue Number: None Related PR: apache#59263 Problem Summary: Extract lazy-pruned column recovery in SegmentIterator into a private helper so the internal rowid mapping, lazy read phase, read_by_rowids invocation, and finalize_lazy_phase behavior can be tested directly. Add white-box UT coverage for non-empty filtered selection and empty selection finalize path. Fix ColumnReader NULL_MAP_ONLY read_by_rowids so a no-null page only fills the sparse rowids that actually belong to the current page. Keep predicate NULL/OFFSET paths out of final allAccessPaths when regular data paths also exist, while preserving them in predicateAccessPaths, so mixed-version BEs do not switch a mixed data/meta read to current-level metadata-only mode. Also remove the racy regression-test profile cleanup and cover lazy-pruned output recovery through a profile-token SQL shape. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test - ./run-be-ut.sh --run --filter=SegmentIteratorLazyPrunedTest.* - ./run-be-ut.sh --run --filter=ColumnReaderTest.* - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.rewrite.PruneNestedColumnTest - /mnt/disk7/hushenggang/.codex/skills/doris-local-regression/scripts/doris-local-regression.sh --network 10.26.20.3/24 all -d datatype_p0/complex_types -s test_pruned_columns - Behavior changed: No - Does this need documentation: No
Member
Author
|
/review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
The subcolumns of a pruned complex-type column can fall into two categories:
For non-predicate columns, Doris can defer reading them until after predicate evaluation, reducing unnecessary I/O for nested columns.
This update also preserves predicate metadata paths separately from final lazy-materialization data paths. Predicate evaluation may still need current-level metadata, such as
OFFSETforcardinality()/length()andNULLforIS NULL, even when a covering data path is also needed later. The BE nested column iterators consume those current-level metadata paths at the correct iterator level without forwarding them to child iterators or incorrectly switching mixed data reads into metadata-only mode.This PR also handles predicate-only nested access paths. A complex iterator should skip access-path setup only when both final access paths and predicate access paths are empty. Otherwise predicate-only child or metadata paths may be ignored in lazy read mode, causing predicates such as array element
IS NULLto evaluate on unread placeholder data.This PR also removes references to
olap/rowset/segment_v2/column_reader.hfrom many header files, avoiding large-scale recompilation of source files caused by changes toColumnReader/ColumnIterator.Issue Number: None
Related PR: None
Release note
For non-predicate columns, Doris can defer reading them until after predicate evaluation, which may significantly reduce the amount of data read.
Check List (For Author)
Test
doris-local-regression --network 10.26.20.3/24 run -d nereids_rules_p0/column_pruning -s string_length_column_pruningdoris-local-regression --network 10.26.20.3/24 run -d datatype_p0/complex_types -s test_pruned_columnsdoris-local-regression --network 10.26.20.3/24 run -d inverted_index_p0/array_contains -s test_index_compaction_null_arr./run-be-ut.sh --run --filter=ColumnReaderTest.*build-support/clang-format.shgit diff --checkBehavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)